ENG-1187: Prod: Implement get and set for block prop based settings #669

sid597 · 2026-01-05T08:58:33Z

Summary by CodeRabbit

New Features
- Added comprehensive settings management infrastructure supporting feature flags, global settings, user-specific settings, and discourse node configurations with robust data validation and nested property access.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

linear · 2026-01-05T08:58:36Z

ENG-1187 Base: Accessors: implement Get and Set setttings as block properties functions

supabase · 2026-01-05T08:58:40Z

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

sid597 · 2026-01-05T08:58:53Z

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

ENG-1272 Migrate all small global settings components #688 : 2 dependent PRs (#690 , #691 )
ENG-1274: Migrate feature flags #684
ENG-1190 Replace existing roam-js components with blockProp based ones #689
ENG-1188: prod: pull watchers for prop based settings #682
ENG-1187: Prod: Implement get and set for block prop based settings #669 👈 (View in Graphite)
ENG-1189: Prod: Init block prop based schema #666
ENG-1105: zod schema for all the settings #664
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

sid597 · 2026-01-05T08:59:24Z

@CodeRabbit ai full review

coderabbitai · 2026-01-05T08:59:39Z

@sid597: I'll perform a full review of this PR now.

🧠 Learnings used

Learnt from: sid597
Repo: DiscourseGraphs/discourse-graph PR: 630
File: apps/roam/src/utils/settingsUsingBlockProps.ts:64-66
Timestamp: 2025-12-22T05:43:09.803Z
Learning: In `apps/roam/src/utils/settingsUsingBlockProps.ts`, the `setBlockPropBasedSettings` function expects callers to ensure that the `value` parameter matches the expected type for the given `keys` path. Type validation is handled at the caller side rather than within the utility function.

✅ Actions performed

Full review triggered.

coderabbitai · 2026-01-05T09:02:52Z

📝 Walkthrough

Walkthrough

This PR introduces a new utility module for managing Roam block properties with typed, schema-validated helpers for reading and writing feature flags, global settings, personal settings, and discourse node configurations. It also exports previously internal functions from the init module as public APIs.

Changes

Cohort / File(s)	Summary
New Settings Accessors Module `apps/roam/src/components/settings/utils/accessors.ts`	New file with 15+ exported functions providing typed read/write access to nested block properties. Includes: basic block prop accessors (getBlockPropsByUid, setBlockPropsByUid), feature flags management (getFeatureFlags, getFeatureFlag, setFeatureFlag), global settings (getGlobalSettings, getGlobalSetting, setGlobalSetting), personal settings (getPersonalSettings, getPersonalSetting, setPersonalSetting), and discourse node settings (getDiscourseNodeSettings, getDiscourseNodeSetting, setDiscourseNodeSetting, getAllDiscourseNodes). All functions integrate Zod schema validation and handle nested key traversal with non-destructive updates.
Init Module Public Exports `apps/roam/src/components/settings/utils/init.ts`	Exports getPersonalSettingsKey and getDiscourseNodePageTitle as public functions. Adds new public helper getDiscourseNodePageUid to compute page UIDs for discourse nodes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

DiscourseGraphs/discourse-graph#245: The new accessors module directly depends on and wraps getBlockProps/setBlockProps utilities introduced in this PR.

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: implementing get and set functionality for block property-based settings, which directly aligns with the changeset's core objective.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9167a2f and 1ebeb69.

📒 Files selected for processing (2)

apps/roam/src/components/settings/utils/accessors.ts
apps/roam/src/components/settings/utils/init.ts

🧰 Additional context used

📓 Path-based instructions (5)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/main.mdc)

**/*.{ts,tsx}: Use Tailwind CSS for styling where possible
When refactoring inline styles, use tailwind classes
Prefer type over interface in TypeScript
Use explicit return types for functions
Avoid any types when possible
Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use PascalCase for components and types
Use camelCase for variables and functions
Use UPPERCASE for constants
Function names should describe their purpose clearly
Prefer early returns over nested conditionals for better readability

Files:

apps/roam/src/components/settings/utils/init.ts
apps/roam/src/components/settings/utils/accessors.ts

apps/roam/**/*.{js,ts,tsx,jsx,json}

📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)

Prefer existing dependencies from package.json when working on the Roam Research extension

Files:

apps/roam/src/components/settings/utils/init.ts
apps/roam/src/components/settings/utils/accessors.ts

apps/roam/**/*.{ts,tsx,jsx,js,css,scss}

📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)

Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension

Files:

apps/roam/src/components/settings/utils/init.ts
apps/roam/src/components/settings/utils/accessors.ts

apps/roam/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)

apps/roam/**/*.{ts,tsx,js,jsx}: Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality

Files:

apps/roam/src/components/settings/utils/init.ts
apps/roam/src/components/settings/utils/accessors.ts

apps/roam/**

📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)

Implement the Discourse Graph protocol in the Roam Research extension

Files:

apps/roam/src/components/settings/utils/init.ts
apps/roam/src/components/settings/utils/accessors.ts

🧠 Learnings (12)

📓 Common learnings

Learnt from: sid597
Repo: DiscourseGraphs/discourse-graph PR: 630
File: apps/roam/src/utils/settingsUsingBlockProps.ts:64-66
Timestamp: 2025-12-22T05:43:09.803Z
Learning: In `apps/roam/src/utils/settingsUsingBlockProps.ts`, the `setBlockPropBasedSettings` function expects callers to ensure that the `value` parameter matches the expected type for the given `keys` path. Type validation is handled at the caller side rather than within the utility function.

📚 Learning: 2025-12-07T20:54:20.007Z

Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 559
File: apps/roam/src/utils/findDiscourseNode.ts:37-39
Timestamp: 2025-12-07T20:54:20.007Z
Learning: In apps/roam/src/utils/findDiscourseNode.ts, the function findDiscourseNodeByTitleAndUid accepts both uid and title parameters where uid is primarily used for cache access (as the cache key) while title is used for the actual matching via matchDiscourseNode. This design reflects the pattern where downstream, the uid is mostly used to fetch the title, so the function caches by uid but matches by title.

Applied to files:

apps/roam/src/components/settings/utils/init.ts
apps/roam/src/components/settings/utils/accessors.ts

📚 Learning: 2025-06-17T23:37:45.289Z

Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 220
File: apps/roam/src/utils/conceptConversion.ts:42-56
Timestamp: 2025-06-17T23:37:45.289Z
Learning: In the DiscourseNode interface from apps/roam/src/utils/getDiscourseNodes.ts, the field `node.type` serves as the UID field rather than having a conventional `node.uid` field. This is an unusual naming convention where the type field actually contains the unique identifier.

Applied to files:

apps/roam/src/components/settings/utils/init.ts

📚 Learning: 2025-06-17T23:37:45.289Z

Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 220
File: apps/roam/src/utils/conceptConversion.ts:42-56
Timestamp: 2025-06-17T23:37:45.289Z
Learning: In the DiscourseNode interface from apps/roam/src/utils/getDiscourseNodes.ts, the field `type` serves as the unique identifier field, not a type classification field. The interface has no `uid` or `id` field, making `node.type` the correct field to use for UID-related operations.

Applied to files:

apps/roam/src/components/settings/utils/init.ts
apps/roam/src/components/settings/utils/accessors.ts

📚 Learning: 2025-06-22T10:40:52.752Z

Learnt from: sid597
Repo: DiscourseGraphs/discourse-graph PR: 232
File: apps/roam/src/utils/getAllDiscourseNodesSince.ts:18-31
Timestamp: 2025-06-22T10:40:52.752Z
Learning: In apps/roam/src/utils/getAllDiscourseNodesSince.ts, the user confirmed that querying for `?title` with `:node/title` and mapping it to the `text` field in the DiscourseGraphContent type is the correct implementation for retrieving discourse node content from Roam Research, despite it appearing to query page titles rather than block text content.

Applied to files:

apps/roam/src/components/settings/utils/init.ts
apps/roam/src/components/settings/utils/accessors.ts

📚 Learning: 2025-11-05T21:57:14.909Z

Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 534
File: apps/roam/src/utils/createReifiedBlock.ts:40-48
Timestamp: 2025-11-05T21:57:14.909Z
Learning: In the discourse-graph repository, the function `getPageUidByPageTitle` from `roamjs-components/queries/getPageUidByPageTitle` is a synchronous function that returns a string directly (the page UID or an empty string if not found), not a Promise. It should be called without `await`.

Applied to files:

apps/roam/src/components/settings/utils/init.ts

📚 Learning: 2025-06-25T22:56:17.522Z

Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-06-25T22:56:17.522Z
Learning: In the Roam discourse-graph system, the existence of the configuration page (identified by DISCOURSE_CONFIG_PAGE_TITLE) and its corresponding UID is a system invariant. The code can safely assume this page will always exist, so defensive null checks are not needed when using `getPageUidByPageTitle(DISCOURSE_CONFIG_PAGE_TITLE)`.

Applied to files:

apps/roam/src/components/settings/utils/init.ts

📚 Learning: 2025-06-23T11:49:45.457Z

Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 220
File: apps/roam/src/utils/conceptConversion.ts:11-40
Timestamp: 2025-06-23T11:49:45.457Z
Learning: In the DiscourseGraphs/discourse-graph codebase, direct access to `window.roamAlphaAPI` is the established pattern throughout the codebase. The team prefers to maintain this pattern consistently rather than making piecemeal changes, and plans to address dependency injection as a global refactor when scheduled.

Applied to files:

apps/roam/src/components/settings/utils/init.ts

📚 Learning: 2025-11-25T00:52:41.934Z

Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality

Applied to files:

apps/roam/src/components/settings/utils/init.ts
apps/roam/src/components/settings/utils/accessors.ts

📚 Learning: 2025-12-22T05:43:09.803Z

Learnt from: sid597
Repo: DiscourseGraphs/discourse-graph PR: 630
File: apps/roam/src/utils/settingsUsingBlockProps.ts:64-66
Timestamp: 2025-12-22T05:43:09.803Z
Learning: In `apps/roam/src/utils/settingsUsingBlockProps.ts`, the `setBlockPropBasedSettings` function expects callers to ensure that the `value` parameter matches the expected type for the given `keys` path. Type validation is handled at the caller side rather than within the utility function.

Applied to files:

apps/roam/src/components/settings/utils/accessors.ts

📚 Learning: 2025-11-25T00:52:41.934Z

Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality

Applied to files:

apps/roam/src/components/settings/utils/accessors.ts

📚 Learning: 2025-11-06T13:48:35.007Z

Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-11-06T13:48:35.007Z
Learning: In apps/roam/src/utils/createReifiedBlock.ts, the `setBlockProps` function does not need await as it is synchronous or fire-and-forget.

Applied to files:

apps/roam/src/components/settings/utils/accessors.ts

🧬 Code graph analysis (2)

apps/roam/src/components/settings/utils/init.ts (1)

apps/roam/src/components/settings/data/blockPropsSettingsConfig.ts (1)

DISCOURSE_NODE_PAGE_PREFIX (6-6)

apps/roam/src/components/settings/utils/accessors.ts (4)

apps/roam/src/utils/getBlockProps.ts (1)

json (1-7)

apps/roam/src/components/settings/data/blockPropsSettingsConfig.ts (3)

DG_BLOCK_PROP_SETTINGS_PAGE_TITLE (3-4)

TOP_LEVEL_BLOCK_PROP_KEYS (8-11)

DISCOURSE_NODE_PAGE_PREFIX (6-6)

apps/roam/src/components/settings/utils/zodSchema.ts (8)

FeatureFlags (242-242)

FeatureFlagsSchema (88-92)

GlobalSettings (251-251)

GlobalSettingsSchema (125-131)

PersonalSettings (258-258)

PersonalSettingsSchema (167-183)

DiscourseNodeSettings (241-241)

DiscourseNodeSchema (43-86)

apps/roam/src/components/settings/utils/init.ts (2)

getPersonalSettingsKey (24-30)

getDiscourseNodePageUid (36-39)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

GitHub Check: Graphite / mergeability_check
GitHub Check: Graphite / mergeability_check
GitHub Check: Graphite / mergeability_check
GitHub Check: Graphite / mergeability_check
GitHub Check: Graphite / mergeability_check

🔇 Additional comments (15)

apps/roam/src/components/settings/utils/init.ts (2)

24-30: LGTM!

The caching pattern for the personal settings key is well-implemented, and the explicit return type follows coding guidelines.

32-39: LGTM!

Both helper functions are concise with explicit return types. The getDiscourseNodePageUid function correctly delegates to getPageUidByPageTitle, which per retrieved learnings returns an empty string if not found—consistent with how callers in accessors.ts handle this case.

apps/roam/src/components/settings/utils/accessors.ts (13)

1-23: LGTM!

Imports are well-organized, leveraging existing dependencies from roamjs-components and internal utilities as per coding guidelines. The Zod schemas and types are properly imported for validation.

25-26: LGTM!

Clean type guard implementation for safely checking record types before property access.

28-53: LGTM!

The nested key traversal logic is correctly implemented with proper null/undefined handling. The early return for empty blockUid is a good defensive pattern.

55-96: LGTM!

The function correctly handles nested key paths, creating intermediate objects as needed. The deep clone via JSON.parse/stringify is appropriate given the json type constraint. Per retrieved learnings, setBlockProps is synchronous/fire-and-forget, so no await is needed.

98-120: LGTM!

Clean implementation using named parameters as per coding guidelines. The pattern of using the first key as block text and remaining keys for nested access is consistent.

122-147: LGTM!

Implementation correctly delegates type validation to callers, consistent with the established pattern per retrieved learnings.

149-172: LGTM!

Feature flag accessors are well-implemented with proper Zod validation. The setFeatureFlag function's explicit boolean validation provides a good safety net.

174-196: LGTM!

Global settings accessors follow the established patterns consistently. The generic <T> type parameter provides flexibility for callers.

198-226: LGTM!

Personal settings accessors correctly use getPersonalSettingsKey() to scope settings per user, maintaining consistency with the global settings pattern.

228-254: LGTM!

Good defensive implementation that handles both UID and label inputs. Using safeParse with warning logging is appropriate for potentially malformed data.

256-268: LGTM!

Consistent pattern with other setting getters, correctly handling undefined base settings.

270-293: LGTM!

The dual lookup pattern correctly handles both UID and label inputs, with appropriate warning when the page cannot be resolved.

295-321: LGTM!

The Datalog query correctly finds all discourse node pages by title prefix. The type override with pageUid aligns with the established pattern where type serves as the unique identifier (per retrieved learnings). Using safeParse gracefully handles any malformed node data.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

sid597 marked this pull request as ready for review January 5, 2026 08:58

sid597 mentioned this pull request Jan 5, 2026

ENG-1105: zod schema for all the settings #664

Open

sid597 mentioned this pull request Jan 5, 2026

ENG-1189: Prod: Init block prop based schema #666

Open

sid597 force-pushed the eng-1187-implement-get-and-set-setttings-as-block-properties-prod branch from 1ebeb69 to 22de4b1 Compare January 7, 2026 18:38

sid597 force-pushed the eng-1189-prod-init-schema branch 2 times, most recently from e30930e to 0ca99fc Compare January 8, 2026 05:18

sid597 force-pushed the eng-1187-implement-get-and-set-setttings-as-block-properties-prod branch from 22de4b1 to 9464473 Compare January 8, 2026 05:18

sid597 force-pushed the eng-1189-prod-init-schema branch from 0ca99fc to 7ea650f Compare January 11, 2026 06:31

sid597 force-pushed the eng-1187-implement-get-and-set-setttings-as-block-properties-prod branch from 9464473 to 3d7e4c7 Compare January 11, 2026 06:31

This was referenced Jan 11, 2026

ENG-1188: prod: pull watchers for prop based settings #682

Open

ENG-1274: Migrate feature flags #684

Open

sid597 force-pushed the eng-1187-implement-get-and-set-setttings-as-block-properties-prod branch from 3d7e4c7 to fb87a45 Compare January 11, 2026 19:54

sid597 mentioned this pull request Jan 12, 2026

ENG-1272 Migrate all small global settings components #688

Open

ENG-1187: Prod: Implement get and set for block prop based settings

3fbeb9f

sid597 force-pushed the eng-1187-implement-get-and-set-setttings-as-block-properties-prod branch from fb87a45 to 3fbeb9f Compare January 12, 2026 15:26

sid597 force-pushed the eng-1189-prod-init-schema branch from 0365026 to 76d487b Compare January 12, 2026 15:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ENG-1187: Prod: Implement get and set for block prop based settings #669

ENG-1187: Prod: Implement get and set for block prop based settings #669

Uh oh!

sid597 commented Jan 5, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

linear bot commented Jan 5, 2026

Uh oh!

supabase bot commented Jan 5, 2026

Uh oh!

sid597 commented Jan 5, 2026 •

edited

Loading

Uh oh!

sid597 commented Jan 5, 2026

Uh oh!

coderabbitai bot commented Jan 5, 2026

Uh oh!

coderabbitai bot commented Jan 5, 2026

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ENG-1187: Prod: Implement get and set for block prop based settings #669

Are you sure you want to change the base?

ENG-1187: Prod: Implement get and set for block prop based settings #669

Uh oh!

Conversation

sid597 commented Jan 5, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

linear bot commented Jan 5, 2026

Uh oh!

supabase bot commented Jan 5, 2026

Uh oh!

sid597 commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sid597 commented Jan 5, 2026

Uh oh!

coderabbitai bot commented Jan 5, 2026

Uh oh!

coderabbitai bot commented Jan 5, 2026

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Pre-merge checks

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sid597 commented Jan 5, 2026 •

edited by coderabbitai bot

Loading

sid597 commented Jan 5, 2026 •

edited

Loading